Skip to content

feat: implement wallpaper cache plugin for dde-services#61

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
yixinshark:feat/wallpaper-cache
Apr 24, 2026
Merged

feat: implement wallpaper cache plugin for dde-services#61
deepin-bot[bot] merged 1 commit into
linuxdeepin:masterfrom
yixinshark:feat/wallpaper-cache

Conversation

@yixinshark

Copy link
Copy Markdown
Contributor

The wallpapercache plugin provides the following main functions:

  • Wallpaper scaling: Asynchronously scales wallpapers to match different screen resolutions and caches the results for performance.
  • Image effects: Synchronously generates processed images (such as Gaussian blur) for the greeter and lock screen.
  • D-Bus API: Provides a unified interface for desktop components to request processed wallpaper paths, supporting both direct file paths and file descriptors (via FD passing).
  • Cache management: Automatically manages and cleans up processed image caches to optimize disk usage.

Log: implement wallpaper cache plugin for dde-services
Pms: TASK-384099

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @yixinshark, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@yixinshark yixinshark force-pushed the feat/wallpaper-cache branch 3 times, most recently from 9aeb6af to 2f800bc Compare April 17, 2026 08:11
The wallpapercache plugin provides the following main functions:
- Wallpaper scaling: Asynchronously scales wallpapers to match different screen resolutions and caches the results for performance.
- Image effects: Synchronously generates processed images (such as Gaussian blur) for the greeter and lock screen.
- D-Bus API: Provides a unified interface for desktop components to request processed wallpaper paths, supporting both direct file paths and file descriptors (via FD passing).
- Cache management: Automatically manages and cleans up processed image caches to optimize disk usage.

Log: implement wallpaper cache plugin for dde-services
Pms: TASK-384099
Change-Id: Ie67988592285bdfb9ec541ac849f5bef264852bd
@yixinshark yixinshark force-pushed the feat/wallpaper-cache branch from 2f800bc to 896f631 Compare April 23, 2026 11:39
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

代码审查报告

1. 语法与逻辑问题

1.1 文件路径处理问题

位置: scaleimagethread.cppcacheImageToDisk 函数

QString format = originalFileInfo.suffix(); // TODO format is null ?
// md5_1920x1080.jpg
QString fileName = md5String + "_" + sizeToString(task.targetSize) + "." + format;

问题:

  1. 当原始文件没有扩展名时,format 将为空,导致生成的文件名缺少扩展名
  2. 代码中已有 TODO 注释表明开发者意识到这个问题,但未解决

建议:

QString format = originalFileInfo.suffix();
if (format.isEmpty()) {
    // 尝试从 QImageReader 获取格式
    QImageReader reader(task.originalPath);
    format = reader.format();
    if (format.isEmpty()) {
        // 默认使用 jpg 格式
        format = "jpg";
    }
}
QString fileName = md5String + "_" + sizeToString(task.targetSize) + "." + format;

1.2 文件描述符处理问题

位置: wallpapercacheservice.cppsaveImageFromFd 函数

char buffer[4096];
ssize_t bytesRead;
while ((bytesRead = read(fdi, buffer, sizeof(buffer))) > 0) {
    if (originFile.write(buffer, bytesRead) != bytesRead) {
        qWarning() << "Failed to write data to destination file.";
        break;
    }
}

问题:

  1. 没有检查 readwrite 的错误情况
  2. 循环中如果 write 失败,会继续读取数据但没有处理

建议:

char buffer[4096];
ssize_t bytesRead;
while ((bytesRead = read(fdi, buffer, sizeof(buffer))) > 0) {
    qint64 bytesWritten = originFile.write(buffer, bytesRead);
    if (bytesWritten != bytesRead) {
        qWarning() << "Failed to write data to destination file. Expected:" 
                   << bytesRead << "Written:" << bytesWritten;
        originFile.close();
        return QString();
    }
}
if (bytesRead < 0) {
    qWarning() << "Error reading from file descriptor:" << strerror(errno);
    originFile.close();
    return QString();
}

1.3 线程安全问题

位置: cachedwallpaper.cppgetCachedImagePaths 函数

QStringList CachedWallpaper::getCachedImagePaths(const QString &originalPath, const QList<QSize> &sizes, bool isMd5Path)
{
    QList<QSize> noCachedSizes;
    QStringList results;

    QString pathMd5 = ScaleImageThread::pathMd5(originalPath, isMd5Path);

    if (m_cachedImages.contains(pathMd5)) {
        const QMap<QString, QString> &map = m_cachedImages[pathMd5];
        // ...
    }
    // ...
}

问题:

  1. m_cachedImages 是一个成员变量,但在多线程环境下访问没有加锁保护
  2. 虽然信号槽使用 Qt::QueuedConnection,但 m_cachedImages 的访问仍然可能存在竞态条件

建议:

QStringList CachedWallpaper::getCachedImagePaths(const QString &originalPath, const QList<QSize> &sizes, bool isMd5Path)
{
    QMutexLocker locker(&m_cacheMutex); // 添加互斥锁保护
    QList<QSize> noCachedSizes;
    QStringList results;

    QString pathMd5 = ScaleImageThread::pathMd5(originalPath, isMd5Path);

    if (m_cachedImages.contains(pathMd5)) {
        const QMap<QString, QString> &map = m_cachedImages[pathMd5];
        // ...
    }
    // ...
}

并在 cachedwallpaper.h 中添加互斥锁成员:

private:
    QMutex m_cacheMutex;

2. 代码质量问题

2.1 魔法数字和硬编码值

位置: imageeffectprocessor.cpp

#define PIXMIX_MATRIX       16          // Image sample size
#define PIXMIX_OPACITY      90          // Color opacity
#define PIXMIX_SATURATION   50          // Saturation
#define PIXMIX_BRIGHTNESS   -60         // Brightness

问题:

  1. 这些参数应该可以通过配置文件或D-Bus接口动态调整
  2. 硬编码的值使得用户无法自定义模糊效果

建议:

  1. 将这些参数移到配置文件中
  2. 添加D-Bus接口允许运行时调整这些参数
  3. 添加参数验证逻辑,确保参数在合理范围内

2.2 错误处理不一致

位置: 多处

问题:

  1. 有些函数在失败时返回空字符串,有些返回false,有些直接返回原始路径
  2. 错误信息不够详细,难以诊断问题

建议:

  1. 统一错误处理策略
  2. 使用枚举类型或错误码表示不同类型的错误
  3. 提供更详细的错误信息,包括错误原因和可能的解决方案

2.3 缺少输入验证

位置: wallpapercacheservice.cpp 的多个函数

QStringList WallpaperCacheService::GetProcessedImagePaths(const QString &originalPath, const QVariantList &sizeArray)
{
    if (!QFile::exists(originalPath)) {
        return QStringList() << originalPath;
    }
    // ...
}

问题:

  1. 没有验证 sizeArray 是否为空
  2. 没有验证尺寸值是否合理(如负值或过大值)

建议:

QStringList WallpaperCacheService::GetProcessedImagePaths(const QString &originalPath, const QVariantList &sizeArray)
{
    if (!QFile::exists(originalPath)) {
        qWarning() << "Original image not exists:" << originalPath;
        return QStringList();
    }

    if (sizeArray.isEmpty()) {
        qWarning() << "Size array is empty";
        return QStringList() << originalPath;
    }

    QList<QSize> sizes = parseSizeArray(sizeArray);
    for (const QSize &size : sizes) {
        if (size.isEmpty() || size.width() <= 0 || size.height() <= 0) {
            qWarning() << "Invalid size:" << size;
            return QStringList() << originalPath;
        }
        if (size.width() > 8192 || size.height() > 8192) {
            qWarning() << "Size too large:" << size;
            return QStringList() << originalPath;
        }
    }
    // ...
}

3. 代码性能问题

3.1 频繁的文件系统访问

位置: cachedwallpaper.cppgetBlurImagePath 函数

QString CachedWallpaper::getBlurImagePath(const QString &originalPath)
{
    QString pathMd5 = QCryptographicHash::hash(originalPath.toUtf8(), QCryptographicHash::Md5).toHex();

    QMutexLocker locker(&m_blurGenerateMutex);

    auto it = m_blurImageCache.constFind(pathMd5);
    if (it != m_blurImageCache.constEnd() && QFile::exists(it.value())) {
        return it.value();
    }
    // ...
}

问题:

  1. 每次调用都会检查文件是否存在,即使内存中已有缓存
  2. 对于频繁访问的图像,这会增加不必要的系统调用

建议:

  1. 添加一个内存标志,记录文件是否已验证存在
  2. 考虑使用文件系统监视器(QFileSystemWatcher)监听缓存文件变化

3.2 图像处理性能

位置: imageeffectprocessor.cppprocessPixmixEffect 函数

QImage ImageEffectProcessor::processPixmixEffect(const QString &imagePath)
{
    QImage originalImage;
    if (!originalImage.load(imagePath)) {
        qWarning() << "Failed to load image:" << imagePath;
        return QImage();
    }

    QColor averageColor = calculateAverageColor(originalImage, PIXMIX_MATRIX);
    // ...
}

问题:

  1. 对于大尺寸图像,直接加载到内存可能消耗大量资源
  2. 没有对图像进行预处理或采样

建议:

QImage ImageEffectProcessor::processPixmixEffect(const QString &imagePath)
{
    // 先获取图像尺寸,避免加载过大图像
    QImageReader reader(imagePath);
    QSize originalSize = reader.size();
    
    // 如果图像过大,先进行采样
    if (originalSize.width() > 3840 || originalSize.height() > 2160) {
        reader.setScaledSize(QSize(1920, 1080)); // 采样到1080p
    }
    
    QImage originalImage;
    if (!reader.read(&originalImage)) {
        qWarning() << "Failed to load image:" << imagePath;
        return QImage();
    }

    QColor averageColor = calculateAverageColor(originalImage, PIXMIX_MATRIX);
    // ...
}

3.3 缓存策略问题

位置: cachedwallpaper.cpp

问题:

  1. 没有缓存大小限制,可能导致磁盘空间耗尽
  2. 没有缓存淘汰策略(LRU等)
  3. 没有缓存清理机制

建议:

  1. 添加缓存大小限制配置
  2. 实现LRU(最近最少使用)缓存淘汰策略
  3. 定期检查缓存大小,超出限制时自动清理最旧的缓存
  4. 添加缓存统计信息接口,便于监控和调试

4. 代码安全问题

4.1 路径遍历漏洞

位置: wallpapercacheservice.cpp 的多个函数

QStringList WallpaperCacheService::GetProcessedImagePaths(const QString &originalPath, const QVariantList &sizeArray)
{
    if (!QFile::exists(originalPath)) {
        return QStringList() << originalPath;
    }
    // ...
}

问题:

  1. 没有验证 originalPath 是否包含路径遍历字符(如 ../
  2. 可能导致访问预期之外的文件

建议:

bool isValidFilePath(const QString &path) {
    // 规范化路径
    QString normalizedPath = QDir::cleanPath(path);
    // 检查是否包含路径遍历
    if (normalizedPath.contains("../") || normalizedPath.contains("..\\")) {
        return false;
    }
    // 检查是否是绝对路径
    QFileInfo fileInfo(normalizedPath);
    if (!fileInfo.isAbsolute()) {
        return false;
    }
    return true;
}

QStringList WallpaperCacheService::GetProcessedImagePaths(const QString &originalPath, const QVariantList &sizeArray)
{
    if (!isValidFilePath(originalPath)) {
        qWarning() << "Invalid file path:" << originalPath;
        return QStringList();
    }
    
    if (!QFile::exists(originalPath)) {
        return QStringList();
    }
    // ...
}

4.2 权限问题

位置: wallpapercacheservice.cppsaveImageFromFd 函数

QString WallpaperCacheService::saveImageFromFd(const QDBusUnixFileDescriptor &fd, const QString &imagePathMd5)
{
    int fdi = fd.fileDescriptor();
    if (fdi <= 0) {
        return QString();
    }

    QString originPath = kWallpaperCacheDir + "/" + imagePathMd5;
    QFile originFile(originPath);
    if (!originFile.open(QIODevice::ReadWrite)) {
        qWarning() << "Failed to open destination file for writing:" << originPath;
        return QString();
    }
    // ...
}

问题:

  1. 没有验证文件描述符的来源和权限
  2. 可能被利用来写入任意位置

建议:

  1. 添加文件描述符来源验证
  2. 确保只能写入预定义的缓存目录
  3. 设置适当的文件权限(如0600)

4.3 资源耗尽攻击

位置: scaleimagethread.cppaddTasks 函数

void ScaleImageThread::addTasks(const QString &originalPath, const QList<QSize> &sizes, bool isMd5Path)
{
    if (!QFile::exists(originalPath)) {
        qWarning() << "file not exists:" << originalPath;
        return;
    }

    QList<TaskData> tasks;
    for (const QSize &size : sizes) {
        TaskData data;
        data.originalPath = originalPath;
        data.targetSize = size;
        data.isMd5Path = isMd5Path;

        tasks.append(data);
    }

    QMutexLocker locker(&m_mutex);
    for (const TaskData &task : tasks) {
        if (!m_tasks.contains(task)) {
            m_tasks.enqueue(task);
        }
    }
    // ...
}

问题:

  1. 没有限制任务队列的大小,可能导致内存耗尽
  2. 没有限制单个请求的尺寸数量,可能被利用进行DoS攻击

建议:

void ScaleImageThread::addTasks(const QString &originalPath, const QList<QSize> &sizes, bool isMd5Path)
{
    if (!QFile::exists(originalPath)) {
        qWarning() << "file not exists:" << originalPath;
        return;
    }

    // 限制单个请求的尺寸数量
    if (sizes.size() > 10) {
        qWarning() << "Too many sizes requested:" << sizes.size();
        return;
    }

    QList<TaskData> tasks;
    for (const QSize &size : sizes) {
        TaskData data;
        data.originalPath = originalPath;
        data.targetSize = size;
        data.isMd5Path = isMd5Path;

        tasks.append(data);
    }

    QMutexLocker locker(&m_mutex);
    
    // 限制任务队列大小
    if (m_tasks.size() > 100) {
        qWarning() << "Task queue is full, rejecting new tasks";
        return;
    }
    
    for (const TaskData &task : tasks) {
        if (!m_tasks.contains(task)) {
            m_tasks.enqueue(task);
        }
    }
    // ...
}

5. 其他建议

5.1 添加日志和监控

  1. 添加更详细的日志记录,包括性能指标
  2. 添加监控接口,便于运维人员监控系统状态
  3. 实现日志轮转,避免日志文件过大

5.2 添加单元测试

  1. 为核心功能添加单元测试
  2. 添加边界条件测试
  3. 添加性能测试,确保系统在高负载下仍能正常工作

5.3 文档改进

  1. 添加更详细的API文档
  2. 添加配置说明和示例
  3. 添加故障排查指南

总结

该代码实现了一个壁纸缓存服务,主要功能包括图像缩放和模糊效果处理。代码整体结构清晰,但存在一些问题需要改进:

  1. 语法与逻辑问题:文件路径处理、文件描述符处理和线程安全问题需要修复
  2. 代码质量问题:减少魔法数字,统一错误处理,加强输入验证
  3. 性能问题:优化文件系统访问,改进图像处理,实现合理的缓存策略
  4. 安全问题:修复路径遍历漏洞,加强权限控制,防止资源耗尽攻击

建议按照优先级逐步解决这些问题,特别是安全相关的问题应该优先处理。同时,建议添加更多的测试和文档,以提高代码的可靠性和可维护性。

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mhduiy, yixinshark

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@yixinshark

Copy link
Copy Markdown
Contributor Author

/forcemerge

@deepin-bot

deepin-bot Bot commented Apr 24, 2026

Copy link
Copy Markdown

This pr force merged! (status: blocked)

@deepin-bot deepin-bot Bot merged commit 058c633 into linuxdeepin:master Apr 24, 2026
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants